Review — PR #845 (round 1)

No confidentiality findings in the changesets, JSDoc, or comments. @anthropic-ai/claude-code / @openai/codex are in the pinned public provider vocabulary.

1. @sapiom/harness changeset is the wrong level and describes work that isn't in the package

.changeset/managed-agent-runtime.md marks @sapiom/harness patch, but the diff adds public API to it:

• export { createCodexAdapter } in src/index.ts — a new export from the package root;
• binaryArgs / binaryEnv on ClaudeCodeAdapterOptions and CodexAdapterOptions — new public option fields;
• a new optional binaryArgs parameter on the exported prepareCodexInferenceProfile / runCodexStructuredInference.

Repo precedent for exactly this is minor (.changeset/durable-delegation-state.md: "Publish … contracts … types" → minor). A consumer on ~0.14.0 gets the new export only by luck of range resolution.

The body is also inaccurate for the package it ships in. @sapiom/harness's published CHANGELOG will claim "isolated coding-agent installs", "Bound installer processes" and "verify packaged CLI launches" — all of that lives in harness-desktop (private: true, never published) — and "private Agent Map initialization", whose only harness change is a rewritten assertion in server/agent-map-mcp-wiring.test.ts. Split the desktop half into its own changeset and drop the Agent Map clause; a changelog line cannot be edited after publish. The house style for a foundation-only harness change is .changeset/bootstrap-coordinator.md ("Internal groundwork … No user-facing behavior changes in this release").

2. ELECTRON_RUN_AS_NODE=1 is exported into every command the agent runs

resolveAgentCommand (harness-desktop/src/main/managed-agent.ts:26) returns the host runtime's binaryEnv ({ELECTRON_RUN_AS_NODE: "1"} per smoke.ts:110), and the adapters now merge it into the session spec env (claude-code.ts:497,510,555, codex.ts:331,342). The PTY agent is the process that runs the user's own build and test commands, and env is inherited wholesale by everything it spawns.

Failure: a user working on an Electron project in Studio; the agent runs npx electron . or electron-builder, and Electron starts as bare Node instead of launching — silently, with no error to trace back. This is the same class the harness already defends against by stripping HOST_ESBUILD_PIN from every child env (packages/harness/CLAUDE.md §2). Existing uses of the flag (codex.ts launchTask, the shim-files.ts shims) scope it to a short-lived controlled subprocess; this is the first time it lands on an interactive session.

Fix: launch the managed JS entry through a wrapper that does delete process.env.ELECTRON_RUN_AS_NODE before requiring the CLI. Electron reads the flag only at process start, so deleting it post-start keeps the CLI running as Node while its children see a clean env.

3. POSIX process-group kill has no PID-reuse guard, unlike Windows

agent-update-process.ts:379 — process.kill(-child.pid, "SIGKILL"). stop() is reachable after the child exited: close only fires once the stdio pipes close, so a surviving grandchild holding the pipes keeps the promise pending until the timeout fires, by which point Node has reaped the child and the OS has freed the PID (exitedAt is set — the code records it but only consults it on Windows).

Failure: the freed PID is reused by a new group leader; kill(-pid) then SIGKILLs an entire unrelated process group belonging to the user. windowsUpdateTree defends against precisely this (identity.exitedAt !== undefined → return [], and the test at agent-update-process.test.ts:104 pins it). The POSIX branch — which the parentExits: true test case exercises — has no equivalent. Low probability on Linux, materially higher on macOS's smaller PID space. Cheapest fix: skip the group kill when exitedAt !== undefined, matching Windows.

4. managed-agent.ts has no unit tests

agent-update-process.ts got 132 lines of tests; resolveAgentCommand got none. Its only exercise is checkManagedAgent in smoke.ts, which runs a single happy path (.cjs bin, lib/node_modules layout) and only inside a packaged Electron build — so it never runs in pnpm test. Untested and load-bearing: the bin-escapes-the-package-dir rejection (managed-agent.ts:56), the Windows node_modules layout, and the native-binary branch that drops runtime.binaryEnv. These are cheap pure-function tests over a temp prefix.

Nit

• The PR body publishes third-party model identifiers observed in a private account catalog ("GPT-6 Astra and GPT-5.6"). That detail proves nothing the version numbers don't; consider trimming it from a public PR.

Verdict

Request changes — the changeset level and body (1) must be fixed before this can publish, and (2) needs a decision before #843 wires it into real sessions.

Review — PR #845 (round 2, follow-up)

Delta reviewed: 3cacd717..1aee7faf (2 commits, 9 files). No new confidentiality issues —
the three changesets name only Sapiom packages and the pinned public providers.

Round-1 findings: all four resolved

1. Changeset level/body — fixed. @sapiom/harness is now minor and its body describes
   only what ships in that package (adapter interpreter args/env, createCodexAdapter, launch
   config across resume/structured inference); the desktop half moved to
   .changeset/desktop-managed-installs.md (patch) and the Agent Map clause is gone.
2. ELECTRON_RUN_AS_NODE leaking into every agent-spawned command — fixed as suggested:
   managed-agent.ts:47 injects --import data:…delete process.env.ELECTRON_RUN_AS_NODE
   ahead of the entry, only when the runtime actually sets the flag, so the CLI still starts
   under Electron-as-Node while its children see a clean env. Covered by
   managed-agent.test.ts (asserts null in both the CLI and its grandchild) and by the
   smoke.ts:107 throw. --import needs Node ≥20.6; Electron 33 is 20.18, and the
   non-Electron path is unaffected.
3. POSIX PID-reuse guard — fixed, and better than asked: an owned supervisor keeps the
   group leader alive until the installer's pipes close, and the group kill is now gated on
   exitedAt === undefined (agent-update-process.ts:202), matching Windows.
   agent-update-process-ownership.test.ts pins it.
4. managed-agent.ts untested — fixed: both node_modules layouts, the native-binary
   branch, and the escaping-bin rejection now have tests that run in pnpm test.

The PR body no longer carries the third-party model identifiers from the round-1 nit.

Note (not blocking)

• The deletion is unconditional for the CLI's descendants, so a managed JS CLI that
  re-executes process.execPath would now boot the Electron GUI rather than Node. Both
  pinned providers avoid it (Codex's bin execs its native binary; Claude Code's self-restart
  is off via DISABLE_AUTOUPDATER=1), but it's the assumption fix(harness-desktop): update Claude and Codex on startup #843 inherits.

I could not execute the desktop suites in this sandbox; the assessment is from the diff and
the assertions it adds.

Verdict: approve — merge per the PR's stated ordering (after #844, before #843).